Skip to content

fix(ui5-toolbar): fix padding variable swap, item spacing, and Toolba…#13593

Closed
PetyaMarkovaBogdanova wants to merge 4 commits into
mainfrom
fix-toolbar-findings
Closed

fix(ui5-toolbar): fix padding variable swap, item spacing, and Toolba…#13593
PetyaMarkovaBogdanova wants to merge 4 commits into
mainfrom
fix-toolbar-findings

Conversation

@PetyaMarkovaBogdanova
Copy link
Copy Markdown
Contributor

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova commented May 27, 2026

Three CSS bugs in ui5-toolbar identified via visual spec analysis against the AI test review.

Changes

  • Toolbar.css — Fix swapped variable names in padding shorthand. padding: 0 right 0 left was referencing --_ui5-toolbar-padding-left for the right slot and vice versa:

    /* Before (wrong) */
padding: 0 var(--_ui5-toolbar-padding-left) 0 var(--_ui5-toolbar-padding-right);

/* After (correct) */
padding: 0 var(--_ui5-toolbar-padding-right) 0 var(--_ui5-toolbar-padding-left);

  • Toolbar-parameters.css — Bump --_ui5-toolbar-item-margin-right from 0.25rem → 0.5rem to match the visual spec.

  • ToolbarItem.css — Remove display: inline-block from :host. This was preventing flex-based responsive sizing for slotted content (e.g. ui5-breadcrumbs) because inline-block constrains the host to shrink-to-content width rather than participating correctly in the toolbar's flex layout.

@PetyaMarkovaBogdanova
fix(ui5-toolbar): fix padding variable swap, item spacing, and Toolba…
da81760 
…rItem display

Agent-Logs-Url: https://github.com/UI5/webcomponents/sessions/4fddc2f7-637b-475b-aa77-cb4a38583e71

Co-authored-by: PetyaMarkovaBogdanova <15124079+PetyaMarkovaBogdanova@users.noreply.github.com>
@cla-assistant
Copy link
Copy Markdown

cla-assistant Bot commented May 27, 2026 •
edited
Loading

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ PetyaMarkovaBogdanova
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@ui5-webcomponents-bot
Copy link
Copy Markdown
Collaborator

ui5-webcomponents-bot commented May 27, 2026 •
edited by sap-ui5-webcomponents-release Bot
Loading

🧹 Preview deployment cleaned up: https://pr-13593--ui5-webcomponents.netlify.app

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview May 27, 2026 11:20 Inactive
kgogov
kgogov requested changes May 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@kgogov kgogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @PetyaMarkovaBogdanova,

It looks like there is a failing test: "Should focus on the last interactive element outside the overflow popover when the overflow button disappears."

The issue is reproducible locally.

@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview May 29, 2026 15:57 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview June 2, 2026 08:41 Inactive
@ui5-webcomponents-bot ui5-webcomponents-bot temporarily deployed to preview June 2, 2026 10:03 Inactive
@sap-ui5-webcomponents-release
Copy link
Copy Markdown

sap-ui5-webcomponents-release Bot commented Jun 4, 2026

🚀 Deployed on https://pr-13593--ui5-webcomponents-preview.netlify.app

@PetyaMarkovaBogdanova
Copy link
Copy Markdown
Contributor Author

PetyaMarkovaBogdanova commented Jun 5, 2026 •
edited
Loading

Covered in #13653

@sap-ui5-webcomponents-release
Copy link
Copy Markdown

sap-ui5-webcomponents-release Bot commented Jun 5, 2026

🧹 Preview deployment cleaned up: https://pr-13593--ui5-webcomponents.netlify.app

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kgogov kgogov kgogov requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@PetyaMarkovaBogdanova @ui5-webcomponents-bot @kgogov